app/composables/userPreferences/useUserPreferencesState.ts (1)

1-9: Align the “read-only” wording with the mutable return value.

The docstring promises read-only access, but the returned ref is writable. Either clarify the wording or enforce immutability so expectations are consistent.

shared/schemas/userPreferences.ts (1)

35-38: Type definitions could drift from schema.

The types AccentColorId and BackgroundThemeId are derived from the constants, whilst ColorModePreference and SearchProvider are manually defined literals. Consider deriving these from the schema for consistency:

♻️ Suggested approach

+import type { InferOutput } from 'valibot'
+
+// Derive types from schemas to prevent drift
+export type ColorModePreference = InferOutput<typeof ColorModePreferenceSchema>
+export type SearchProvider = InferOutput<typeof SearchProviderSchema>
 export type AccentColorId = keyof typeof ACCENT_COLORS.light
 export type BackgroundThemeId = keyof typeof BACKGROUND_THEMES
-export type ColorModePreference = 'light' | 'dark' | 'system'
-export type SearchProvider = 'npm' | 'algolia'

app/composables/useUserPreferencesSync.client.ts (2)

137-144: Route guard uses fire-and-forget pattern which may lose data.

The void flushPendingSync() call allows navigation to proceed without awaiting the save. If the network request fails or is slow, the user may navigate away before their preferences are persisted.

Consider awaiting the flush to ensure data is saved before navigation:

♻️ Suggested fix

   function setupRouteGuard(getPreferences: () => UserPreferences): void {
     router.beforeEach(async (_to, _from, next) => {
       if (hasPendingChanges && isAuthenticated.value) {
-        void flushPendingSync(getPreferences())
+        await flushPendingSync(getPreferences())
       }
       next()
     })
   }

---

32-41: Silent error handling may mask connectivity issues.

fetchServerPreferences catches all errors and returns null, making it indistinguishable from "user has no saved preferences" vs "network/server error". Consider logging errors or updating the sync state to reflect fetch failures.

server/utils/preferences/user-preferences-store.ts (1)

23-35: Minor: Double timestamp assignment in merge.

Line 30 sets updatedAt, then line 33 calls this.set() which sets updatedAt again on line 18. This is functionally correct but slightly redundant.

♻️ Optional simplification

   async merge(did: string, partial: Partial<UserPreferences>): Promise<UserPreferences> {
     const existing = await this.get(did)
     const base = existing ?? { ...DEFAULT_USER_PREFERENCES }

     const merged: UserPreferences = {
       ...base,
       ...partial,
-      updatedAt: new Date().toISOString(),
     }

-    await this.set(did, merged)
+    await this.storage.setItem(did, {
+      ...merged,
+      updatedAt: new Date().toISOString(),
+    })
     return merged
   }

app/composables/useUserPreferencesProvider.ts (1)

77-95: Avoid echoing server‑loaded prefs back to the server.
In the auth watcher, assigning preferences.value triggers the deep watch and schedules a sync, even though the data just came from the server. This creates redundant PUTs and can churn timestamps. Consider suppressing scheduleSync while applying server prefs.

♻️ Suggested refactor

   const isSyncing = computed(() => status.value === 'syncing')
   const isSynced = computed(() => status.value === 'synced')
   const hasError = computed(() => status.value === 'error')
+  let isApplyingServerPrefs = false

   async function initSync(): Promise<void> {
@@
     watch(
       preferences,
       newPrefs => {
-        if (isAuthenticated.value) {
+        if (isAuthenticated.value && !isApplyingServerPrefs) {
           scheduleSync(newPrefs)
         }
       },
       { deep: true },
     )

     watch(isAuthenticated, async newIsAuth => {
       if (newIsAuth) {
         const serverPrefs = await loadFromServer()
         if (serverPrefs) {
           const merged = { ...defaultValue, ...preferences.value, ...serverPrefs }
           if (!arePreferencesEqual(preferences.value, merged)) {
-            preferences.value = merged
+            isApplyingServerPrefs = true
+            preferences.value = merged
+            isApplyingServerPrefs = false
           }
         }
       }
     })

app/pages/settings.vue (1)

66-100: Remove the non‑essential template comment.
Line 66 is descriptive rather than explaining complex logic; consider dropping it to keep templates lean.

♻️ Suggested tidy‑up

-        <!-- Sync status indicator for authenticated users -->

As per coding guidelines: “Add comments only to explain complex logic or non-obvious implementations”.

test/nuxt/components/HeaderConnectorModal.spec.ts (1)

104-106: Reset the whole mockUserLocalSettings object to avoid cross‑test leakage.
Only connector is reset; if future tests mutate sidebar, state will bleed between tests.

Proposed fix

 function resetMockState() {
   mockState.value = {
     connected: false,
     connecting: false,
     npmUser: null,
     avatar: null,
     operations: [],
     error: null,
     lastExecutionTime: null,
   }
-  mockUserLocalSettings.value.connector = {
-    autoOpenURL: false,
-  }
+  mockUserLocalSettings.value = {
+    sidebar: {
+      collapsed: [],
+    },
+    connector: {
+      autoOpenURL: false,
+    },
+  }
 }